-
Notifications
You must be signed in to change notification settings - Fork 56
fix: solver session settings access in tests and examples #4512
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
# /// script | ||
# dependencies = [ | ||
# "pyfluent", | ||
# "ansys-fluent-visualization", | ||
# "matplotlib", | ||
# ] | ||
# /// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per the discussion we think that the dependencies are not versioned which is also problematic. I think we should remove this change from all example scripts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hpohekar It looks like this is fairly common practice. I think that the pyfluent
dependency should be given as ansys-fluent-core
though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can specify the dependencies required to run all examples in a section of pyproject.toml as follows:
[project.optional-dependencies]
doc = [
...
]
tests = [
...
]
additional = [
...
]
examples = [
...
]
We can install them as follows:
pip install ansys-fluent-core[examples]
This will remove duplicate boilerplate from all example scripts and helpful from the maintenance point of view.
We can add an instruction in docs to execute pip install ansys-fluent-core[examples]
before running any example scripts locally.
cc: @mkundu1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe @Gobot1234’s intent is to consider the real-world user experience: someone who already has PyFluent installed, comes across this script in isolation, and simply wants to know what additional packages are required to run it successfully.
If this script were circulating outside the repository, having the dependency list inside the script itself would be helpful and self-explanatory.
The question then is: at what point does it stop being useful to include that information? If we remove it purely because the script is now “in the repo,” we risk making it less accessible when encountered independently.
I’d argue that keeping lightweight dependency metadata in examples is a net positive for usability unless we have a stronger alternative convention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it makes sense, considering the points of circulating script outside the repository and its independent accessibility.
I have also verified that Sphinx recognizes this format and does not render it in the generated examples gallery.
cc: @Gobot1234
############################################################################### | ||
# Confirm and update boundaries | ||
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
# Confirm and update the boundaries. | ||
|
||
meshing_session.workflow.TaskObject["Update Boundaries"].Execute() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
############################################################################### | |
# Confirm and update boundaries | |
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | |
# Confirm and update the boundaries. | |
meshing_session.workflow.TaskObject["Update Boundaries"].Execute() |
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
# Confirm and update the boundaries. | ||
|
||
meshing_session.workflow.TaskObject["Update Boundaries"].Execute() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looked sus (moving the code around)
meshing_session.workflow.TaskObject["Update Boundaries"].Execute() | |
############################################################################### | |
# Confirm and update boundaries | |
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | |
# Confirm and update the boundaries. | |
meshing_session.workflow.TaskObject["Update Boundaries"].Execute() |
<solver_session>.file.ead (Command) | ||
<solver_session>.file.mport_.read (Command) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<solver_session>.file.ead (Command) | |
<solver_session>.file.mport_.read (Command) | |
<solver_session>.file.read (Command) | |
<solver_session>.file.import_.read (Command) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
||
pyfluent.config.print_search_results = False | ||
results = pyfluent.search("local*", api_path="<solver_session>.setup") | ||
results = pyfluent.search("local*", api_path="<solver_session>.setup.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
results = pyfluent.search("local*", api_path="<solver_session>.setup.") | |
results = pyfluent.search("local*", api_path="<solver_session>.setup") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
161dd03
to
376508f
Compare
Definitely needs testing cause it was a big find and replace job